Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

カスタムエラーハンドラーをプラグイン化して循環参照を防ぐ #1361

Conversation

KentaHizume
Copy link
Contributor

@KentaHizume KentaHizume commented Oct 10, 2024

注意

マイルストーンv1.0までのフロントエンドの変更をMaiaに寄せるために、
レビュー中にMaris⇒MaiaへPRの移動を行いました。

途中のコメントはMarisの下記のPRを参照願います。

このプルリクエストで実施したこと

  • カスタムエラーをハンドリングするerrorHandler 関数をVueのプラグインに書き換えました。
  • 既存のグローバルエラーハンドラーの名称をglobal-error-handlerに変更し、
    責務を明確化するとともにカスタムエラーハンドラーと形式を揃えました。

モチベーション

  • エラーハンドラーから直接routerの実装をimportしたうえで、画面からエラーハンドラーをimportすると、動作自体は期待通りになるが、循環参照になってしまうという問題があります。
    (例)CataogView⇒ErrorHandler⇒router⇒catalogRoutes⇒CatalogView

  • プラグインを使ったprovide/injectを用いることで、この問題を解決します。
    具体的には、
    App(親コンポーネント)でハンドラーをInjectionKeyに紐づけてprovideし、
    各画面(子コンポーネント)ではInjectionKeyを経由してInjectすることで、循環参照を避けることができます。

確認した点

注文処理実行時に401エラーが発生した際、想定通りログイン画面にルーティングされるよう修正されたことを確認しました。

下記のコメントのケースについて、修正前と動作が変わらないことを確認しました。

グローバルエラーハンドラーについて、修正前と同様、
想定外エラーの発生時にコンソールにログ出力してエラーページに遷移することを確認しました。

このプルリクエストでは実施していないこと

ドキュメントの修正については別issue、PRにて対応いたします。
AzureADB2Cサンプルへの修正については別issue、別PRにて対応いたします。

Issue や Discussions 、関連する Web サイトなどへのリンク

プラグインの実装形式

プラグインの実装形式については、
vue-routerpiniaを参考にしました。

inject時の非NULLアサーションの使用

injectする際に非NULLアサーションを使用しています。
return inject(customErrorHandlerKey)!;
対応するKeyのリソースをprovideしていなかった場合にNULLとなるためです。
そのため、main.tsでapp.useを書き忘れた場合は実行時エラーになります。

ただし、
・テスト時に確実に発覚する
・vue-routerでも同様の実装となっている
という理由から、特別処理する必要はないと考えます。

https://github.com/vuejs/router/blob/main/packages/router/src/useApi.ts#L12

プラグインが役立つユースケース

https://ja.vuejs.org/guide/reusability/plugins#introduction

グローバルエラーハンドラーは[app.config.globalProperties](https://ja.vuejs.org/api/application.html#app-config-globalproperties) にグローバルなインスタンスプロパティやメソッドを追加する。に対応し、
カスタムエラーハンドラーは[app.provide()](https://ja.vuejs.org/api/application.html#app-provide) を呼び出して、アプリケーション全体でリソースを[注入できる](https://ja.vuejs.org/guide/components/provide-inject.html)ようにする。に対応します。

useで始まる名称の使用

カスタムエラーハンドラーはuseで始まる名称の関数でラップしています。
このことにより、カスタムエラーハンドラーを利用する子コンポーネントから、inject()構文を隠蔽することができます。
一方でグローバルエラーハンドラーは子コンポーネントから明示的に呼び出すことがないので、
特にラップする必要はないと考えます。

use~はコンポーザブル関数に対する命名の慣例とされています。

Vue アプリケーションの文脈で「コンポーザブル(composable)」とは、Vue の Composition API を活用して状態を持つロジックをカプセル化して再利用するための関数です。
https://ja.vuejs.org/guide/reusability/composables#conventions-and-best-practices
コンポーザブル関数には "use" で始まるキャメルケースの名前をつけるのが慣例です。

@KentaHizume KentaHizume self-assigned this Oct 10, 2024
@KentaHizume KentaHizume added target: Dressca サンプルアプリケーションDresscaに関係がある サンプルAP labels Oct 10, 2024
@KentaHizume KentaHizume added this to the v0.9.1 milestone Oct 10, 2024
@KentaHizume KentaHizume linked an issue Oct 10, 2024 that may be closed by this pull request
@tsuna-can-se
Copy link
Contributor

対象にAzure AD B2Cサンプルがないですけど、これは認識通りですか?

Copy link
Contributor

@1nu1taichi 1nu1taichi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marisの方では error-handler.ts を削除しているようです。こちらも custom-error-handler.ts に置き換わっていると思うので削除してください。

@1nu1taichi
Copy link
Contributor

1nu1taichi commented Oct 18, 2024

出来ればグローバルエラーハンドラーもカスタムエラーハンドラーと形式をそろえたいです。
私の実装不備を押し付けるようで申し訳ないですが、error-handler-plugin.ts → global-error-hander.ts + use-global-error-handler.ts の形式に修正いただけますでしょうか。

カスタムエラーハンドラーとグローバルエラーハンドラーで用途が違うため、use-の形式はグローバルエラーハンドラーでは不要でした。
処理形式は変更せず名称のみ error-handler-plugin.ts → global-error-hander-plugin.ts とするのはいかがでしょうか。

@tsuna-can-se tsuna-can-se modified the milestones: v0.9.2, v1.0 Oct 23, 2024
@KentaHizume KentaHizume changed the title カスタムエラーハンドラーをプラグイン化して循環参照を防ぐ エラーハンドラーの設定に関するドキュメントを追加する Oct 23, 2024
@KentaHizume
Copy link
Contributor Author

些末な内容で恐縮ですが、先に下記について修正させてください。

@KentaHizume
Copy link
Contributor Author

対象にAzure AD B2Cサンプルがないですけど、これは認識通りですか?

@tsuna-can-se
ご指摘いただきありがとうございます。
別issueを作成して対応するようにいたします。

@KentaHizume KentaHizume changed the title エラーハンドラーの設定に関するドキュメントを追加する カスタムエラーハンドラーをプラグイン化して循環参照を防ぐ Oct 24, 2024
@KentaHizume
Copy link
Contributor Author

@1nu1taichi
お待たせして申し訳ありません、
・いただいたコメントに関する修正
・修正経緯の追記
・動作確認
・ドキュメント部分の修正の分離
が完了いたしましたので、
お手数ですが再度お目通しお願いします。

@1nu1taichi
Copy link
Contributor

@KentaHizume
コードレビュー、動作確認完了しました。追加の指摘はありません。

@tsuna-can-se tsuna-can-se modified the milestones: v1.0, v0.9.1 Oct 28, 2024
@tsuna-can-se tsuna-can-se merged commit c28110c into main Oct 28, 2024
2 checks passed
@tsuna-can-se tsuna-can-se deleted the feature/カスタムエラーハンドラーをプラグイン化して循環参照を防ぐ branch October 28, 2024 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
target: Dressca サンプルアプリケーションDresscaに関係がある
Projects
None yet
Development

Successfully merging this pull request may close these issues.

エラーハンドラーからrouterを参照できない
3 participants